From 4eb28fc4824cee39cc7db524dced51a0b22042c8 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Mon, 17 Jul 2017 13:10:47 +0300 Subject: [PATCH] Allow `src/bench.rs` as a benchmark for legacy reasons closes #4287 cc #3947 --- src/cargo/util/toml/targets.rs | 85 ++++++++++++++++++++++++---------- tests/bench.rs | 31 +++++++++++++ 2 files changed, 91 insertions(+), 25 deletions(-) diff --git a/src/cargo/util/toml/targets.rs b/src/cargo/util/toml/targets.rs index 42f317ace..d7e6af08c 100644 --- a/src/cargo/util/toml/targets.rs +++ b/src/cargo/util/toml/targets.rs @@ -51,7 +51,7 @@ pub fn targets(manifest: &TomlManifest, ); targets.extend( - clean_benches(manifest.bench.as_ref(), package_root)? + clean_benches(manifest.bench.as_ref(), package_root, warnings)? ); // processing the custom build script @@ -175,26 +175,23 @@ fn clean_bins(toml_bins: Option<&Vec>, let mut result = Vec::new(); for bin in bins.iter() { - let path = match target_path(bin, &inferred, "bin", package_root) { - Ok(path) => path, - Err(e) => { - if let Some(legacy_path) = legacy_bin_path(package_root, &bin.name(), has_lib) { - { - let short_path = without_prefix(&legacy_path, package_root) - .unwrap_or(&legacy_path); - - warnings.push(format!( - "path `{}` was erroneously implicitly accepted for binary {},\n\ - please set bin.path in Cargo.toml", - short_path.display(), bin.name() - )); - } - legacy_path - } else { - return Err(e); + let path = target_path(bin, &inferred, "bin", package_root, &mut |_| { + if let Some(legacy_path) = legacy_bin_path(package_root, &bin.name(), has_lib) { + { + let short_path = without_prefix(&legacy_path, package_root) + .unwrap_or(&legacy_path); + + warnings.push(format!( + "path `{}` was erroneously implicitly accepted for binary {},\n\ + please set bin.path in Cargo.toml", + short_path.display(), bin.name() + )); } + Some(legacy_path) + } else { + None } - }; + })?; let mut target = Target::bin_target(&bin.name(), path, bin.required_features.clone()); @@ -263,10 +260,30 @@ fn clean_tests(toml_tests: Option<&Vec>, } fn clean_benches(toml_benches: Option<&Vec>, - package_root: &Path) -> CargoResult> { - let targets = clean_targets("benchmark", "bench", - toml_benches, inferred_benches(package_root), - package_root)?; + package_root: &Path, + warnings: &mut Vec) -> CargoResult> { + let mut legacy_bench_path = |bench: &TomlTarget| { + let legacy_path = package_root.join("src").join("bench.rs"); + if !(bench.name() == "bench" && legacy_path.exists()) { + return None; + } + { + let short_path = without_prefix(&legacy_path, package_root) + .unwrap_or(&legacy_path); + + warnings.push(format!( + "path `{}` was erroneously implicitly accepted for benchmark {},\n\ + please set bench.path in Cargo.toml", + short_path.display(), bench.name() + )); + } + Some(legacy_path) + }; + + let targets = clean_targets_with_legacy_path("benchmark", "bench", + toml_benches, inferred_benches(package_root), + package_root, + &mut legacy_bench_path)?; let mut result = Vec::new(); for (path, toml) in targets { @@ -284,6 +301,19 @@ fn clean_targets(target_kind_human: &str, target_kind: &str, inferred: Vec<(String, PathBuf)>, package_root: &Path) -> CargoResult> { + clean_targets_with_legacy_path(target_kind_human, target_kind, + toml_targets, + inferred, + package_root, + &mut |_| None) +} + +fn clean_targets_with_legacy_path(target_kind_human: &str, target_kind: &str, + toml_targets: Option<&Vec>, + inferred: Vec<(String, PathBuf)>, + package_root: &Path, + legacy_path: &mut FnMut(&TomlTarget) -> Option) + -> CargoResult> { let toml_targets = match toml_targets { Some(targets) => targets.clone(), None => inferred.iter().map(|&(ref name, ref path)| { @@ -302,7 +332,7 @@ fn clean_targets(target_kind_human: &str, target_kind: &str, validate_unique_names(&toml_targets, target_kind)?; let mut result = Vec::new(); for target in toml_targets { - let path = target_path(&target, &inferred, target_kind, package_root)?; + let path = target_path(&target, &inferred, target_kind, package_root, legacy_path)?; result.push((path, target)); } Ok(result) @@ -428,7 +458,8 @@ fn configure(toml: &TomlTarget, target: &mut Target) { fn target_path(target: &TomlTarget, inferred: &[(String, PathBuf)], target_kind: &str, - package_root: &Path) -> CargoResult { + package_root: &Path, + legacy_path: &mut FnMut(&TomlTarget) -> Option) -> CargoResult { if let Some(ref path) = target.path { // Should we verify that this path exists here? return Ok(package_root.join(&path.0)); @@ -444,6 +475,10 @@ fn target_path(target: &TomlTarget, match (first, second) { (Some(path), None) => Ok(path), (None, None) | (Some(_), Some(_)) => { + if let Some(path) = legacy_path(target) { + return Ok(path); + } + bail!("can't find `{name}` {target_kind}, specify {target_kind}.path", name = name, target_kind = target_kind) } diff --git a/tests/bench.rs b/tests/bench.rs index ec2374a0f..2fbbd1f99 100644 --- a/tests/bench.rs +++ b/tests/bench.rs @@ -1158,3 +1158,34 @@ fn bench_all_virtual_manifest() { .with_stdout_contains("test bench_foo ... bench: [..]")); } +// https://github.com/rust-lang/cargo/issues/4287 +#[test] +fn legacy_bench_name() { + if !is_nightly() { return } + + let p = project("foo") + .file("Cargo.toml", r#" + [project] + name = "foo" + version = "0.1.0" + + [[bench]] + name = "bench" + "#) + .file("src/lib.rs", r#" + pub fn foo() {} + "#) + .file("src/bench.rs", r#" + #![feature(test)] + extern crate test; + + use test::Bencher; + + #[bench] + fn bench_foo(_: &mut Bencher) -> () { () } + "#); + + assert_that(p.cargo_process("bench"), execs().with_status(0).with_stderr_contains("\ +[WARNING] path `src[/]bench.rs` was erroneously implicitly accepted for benchmark bench, +please set bench.path in Cargo.toml")); +} -- 2.30.2